Skip to content

feat: adaptive layout system — size-aware regions, crisp font ladders, image fitting#393

Open
ChuckBuilds wants to merge 14 commits into
mainfrom
feat/adaptive-layout
Open

feat: adaptive layout system — size-aware regions, crisp font ladders, image fitting#393
ChuckBuilds wants to merge 14 commits into
mainfrom
feat/adaptive-layout

Conversation

@ChuckBuilds

@ChuckBuilds ChuckBuilds commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Summary

Core infrastructure for plugins that scale to any panel size instead of being authored for one:

  • src/adaptive_layout.pyRegion rect algebra (bands/columns/splits/offset), LayoutContext with breakpoint tiers and a geometry scale factor vs. the manifest's display.design_size, and font fitting against verified-crisp font ladders (measure_font_crispness proves each rung renders with zero antialiasing; PressStart2P is only crisp at exact multiples of 8px — classic football's fixed 10px score has always been 74.6% antialiased, a pre-existing surprise this tooling caught).
  • fit_text_proportional — sizes text relative to its classic design size (nearest crisp rung), not "largest that fits its box," so elements can't balloon out of proportion to their neighbors; optional axis-specific scale override for compositions that scale by height alone.
  • scoreboard_regions() / media_row() — shared composition helpers. scoreboard_regions fixes the logo_slot = min(h, w//2) blind spot at exactly 2:1 aspect ratios (96x48, 128x64, 256x128 — where the logos mathematically claimed the whole card, leaving zero pixels for the score) with a guaranteed center reserve + broadcast-style score bleed. Fixed once here; adopting plugins need zero code changes.
  • src/adaptive_images.pyfit_image (contain/cover/fill_height/stretch, crop-to-ink, NEAREST/LANCZOS) + size-keyed LRU cache on LayoutContext; retires ~15 per-plugin copies of the thumbnail/resample idiom and the size-blind logo caches.
  • BasePlugin.layout / draw_fit / draw_image — one-liner adoption path.
  • Harness: fill/scale-up heuristic (warn-only; strict via harness.json), config variants for dual classic/adaptive goldens, dev-server multi-size gallery.

Verification

  • 64 unit tests in test/test_adaptive_layout.py + test_adaptive_images.py suites; all existing suites pass.
  • Adopted by text-display v1.1.1, football-scoreboard v2.8.x, ledmatrix-music v1.1.x (separate monorepo PRs, all opt-in with classic default and byte-identical classic goldens).
  • Tested on real hardware (192x48 devpi panel).

🤖 Generated with Claude Code

https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam

Summary by CodeRabbit

  • New Features
    • Added adaptive layout & image fitting support for plugins, including composite helpers for common scoreboard/media layouts.
    • Added manifest support for authored design size plus min/max display-size validation.
    • Enhanced Dev Preview with an “All Sizes” multi-panel render gallery.
    • Added render “fill / scale-up” validation for stricter panel coverage checks.
    • Added Dev Preview render APIs for listing sizes and rendering size matrices.
    • Updated the release version to 3.1.0.
  • Documentation
    • Added adaptive layout documentation and examples across plugin development guides and API references.
  • Bug Fixes
    • Fixed logo caching to be size-qualified so resized logos don’t get reused incorrectly.
    • Added core/plugin version compatibility warnings during plugin loading.
  • Tests
    • Added coverage for adaptive layout/images behavior and harness fill validation.

Chuck and others added 9 commits July 11, 2026 09:00
Add src/adaptive_layout.py — opt-in core helpers so plugins render
legibly on any panel size without hand-tuned per-display layouts:

- Region: integer rect algebra (bands/columns/weighted splits/centering)
  that partitions space so text bands can't overlap by construction
- Font ladders: ordered (family, size) steps known to render crisply
  (LADDER_GRID: X11 BDFs at native sizes; LADDER_ARCADE: PressStart2P at
  8px multiples) — fitting walks the ladder instead of scaling pixel
  fonts fractionally
- LayoutContext: breakpoint tiers, geometry scale vs. a declared design
  size, and cached fit_text/fit_lines/font_for_rows queries

Generalizes the three patterns proven in the field: f1-scoreboard's
scale factor, masters-tournament's tiers, baseball-scoreboard's font
fallback ladder.

Wiring: BasePlugin gains a lazy .layout property and draw_fit();
FontManager gains get_native_bdf_size() and a cache_generation counter;
manifest schema gains display.design_size and requires.display_size
max_width/max_height; 96x48 joins DEFAULT_TEST_SIZES; the bounds-check
harness records negative-coordinate draws; TextHelper's broken
measurement helpers are fixed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add src/adaptive_images.py — the image counterpart to fit_text:
- fit_image(img, box, mode=contain|cover|fill_height|stretch,
  crop_to_ink, anchor, resample, upscale) promoting the proven plugin
  patterns (football's crop-to-ink fill-height logos, masters' cover
  crop + NEAREST flags, static-image's letterbox). Upscales by default —
  thumbnail()'s downscale-only behavior is why imagery stays tiny on
  big panels.
- draw_fitted_image() pastes aligned within a Region with alpha mask.
- One central Pillow>=9.1 RESAMPLE shim replacing ~15 plugin copies.

LayoutContext.fit_image() caches results per (identity, box size,
options) with a 64-entry LRU; id()-keyed entries pin the source image.
BasePlugin.draw_image() is the one-liner adoption path beside draw_fit.

Composites in adaptive_layout.py: Region.offset() (user x/y-offset
passthrough), scoreboard_regions() (the two-logos-plus-score card math
duplicated across six sports plugins, logo_slot = min(H, W//2)), and
media_row() (art-left/text-right).

Fix LogoHelper's size-blind cache key (stale sizes on panel change);
deprecation note on dead image_utils.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…allery

Quality gates for adaptive layout:

- fill_metrics()/check_scale_up() in the safety harness: overflow catches
  content too big for a panel, but nothing caught content that stays tiny
  on panels >= 2x the plugin's declared design size. The check measures
  lit-content extents and warns (or fails, when a plugin opts into
  "fill_check": "strict" in test/harness.json) below 50% coverage on the
  doubled axis. Warn-only by default so no existing plugin breaks.

- harness.json "variants": extra runs with config overlays and their own
  golden dirs, so an opt-in mode (e.g. layout_mode: adaptive) is golden-
  tested beside the classic default. check_plugin.py loops base + variants
  and labels variant results mode@name.

- Dev preview server: GET /api/sizes (harness size sample), POST
  /api/render-matrix (render at up to 12 sizes in one call), size-preset
  dropdown, and an "All Sizes" side-by-side gallery in the preview UI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… warning

Discoverability: re-export the adaptive layout/image API from src.common
(the blessed-helpers package plugin authors already know) — canonical
paths stay src.adaptive_layout / src.adaptive_images so nothing breaks.
Document it in src/common/README.md and cross-link ADAPTIVE_LAYOUT.md
from the developer docs authors actually read (quick reference, API
reference, advanced dev, font manager, dev preview, plugin dev guide);
ADAPTIVE_LAYOUT.md gains adaptive-images, composite-layouts and
preserving-user-customization sections.

Compat: PluginLoader now logs one advisory warning (never raises) when a
plugin's manifest declares a min LEDMatrix version newer than the running
core, checking the min_ledmatrix_version / requires.* / versions[]
spellings found in the wild. Guarded against stale core version numbers.

src/__init__.py __version__ bumped 1.0.0 -> 3.1.0 to match the latest
release tag (v3.1.0) — it had never been updated and the compat check
needs a truthful number. NOTE: verify this matches the intended release
numbering before the next tag.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… blurry

PIL antialiases TTF outlines by default; a 'pixel-style' font only
rasterizes without antialiasing at specific sizes (for PressStart2P:
exact multiples of its 8px design grid). A ladder rung at an unverified
size silently renders blurry on an LED panel — this exact bug shipped in
both text-display's and football-scoreboard's custom TTF ladders
(non-8-multiple PressStart2P sizes, and '5by7.regular'/'4x6-font' at
sizes that were never actually crisp).

measure_font_crispness(font, sample_text) renders the sample and reports
the fraction of ink-bbox pixels that are neither pure black nor pure
white. BDF fonts (real bitmaps) always score 0.0; TTF ladders should be
verified against this before shipping — see the new
TestFontFitting::test_ladder_arcade_is_crisp pattern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ays-maximize

fit_text always picks the largest ladder rung that fits its box. That's
right when an element owns dedicated space, but wrong when several
independently-fitted elements need to stay visually harmonious as the
panel grows: a score's box might have generous room while a neighboring
logo scales by a fixed geometry factor via px() — fit_text lets the score
balloon out of proportion (even overlapping the logo) even though its
individual pick is technically correct.

fit_text_proportional(text, box, base_size_px, ladder) instead targets
base_size_px * self.scale (the same scale factor px() already uses),
picking the nearest ladder rung at or below that target, still capped to
what fits the box, floored at the smallest rung when the target is below
every rung. Refactored the shared largest-that-fits/ellipsize walk into
_walk_ladder() so fit_text and fit_text_proportional don't duplicate it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ride

self.scale (min(width_ratio, height_ratio)) is the right conservative
default for anything whose aspect ratio matters, but a caller whose
surrounding composition already scales along a single axis — e.g.
football-scoreboard's logo_slot = min(height, width // 2), which tracks
height alone — needs text sized the same way, or it reads as
under-scaled next to logos that grew on a panel that only got taller
(128x32 -> 128x64: self.scale stays 1.0 since width didn't grow, but
logos still double).

fit_text_proportional(..., scale=None) now accepts an explicit override;
None keeps the existing self.scale default.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ect ratios

logo_slot = min(height, width // 2) has a blind spot: at exactly 2:1
aspect ratio (width == 2 * height -- a very common shape: two, four, or
more square modules stacked into a taller panel) width // 2 and height
are equal, so the two logo slots claim the ENTIRE width and leave zero
pixels for a center column, no matter how large the panel gets. Not a
'small panel' problem -- 96x48, 128x64, and 256x128 (all exactly 2:1) hit
it identically, while the 128x32 design baseline and panels like 192x48
or 256x32 never do, because height is already the tighter constraint
there.

Two new parameters fix it in the one shared helper every scoreboard-style
plugin composes through:

- min_center_fraction / min_center_design_px reserve at least
  max(width * fraction, design_px * ctx.scale) for the center column,
  capping logo_slot further when needed. The scaled design-px term
  matters on small panels where a flat fraction alone reserves too little
  absolute space.
- score_bleed_fraction extends the score's own fit box (not the logo
  slots themselves) a controlled amount into each side -- the same way
  real broadcast scoreboards let a big score number's edges cross into
  the team marks flanking it. Without this the reserve alone can still be
  too narrow for a short score to render without truncating.

score_area is now genuinely narrower than the full card width (previously
identical to status_band/detail_band, which still span the full width and
overlay the logos -- short text there was never the problem).

Verified against the full harness size spread: a real game score like
'17-21' never needs ellipsis at any tested 2:1-or-tighter aspect ratio
(test_score_never_needs_ellipsis_for_a_short_score), and wide panels
(128x32/192x48/256x32-style) are provably unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@ChuckBuilds, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9b12b42-eeda-49a4-bbbe-8a41ddac85c4

📥 Commits

Reviewing files that changed from the base of the PR and between 72b443d and a11c596.

📒 Files selected for processing (1)
  • scripts/dev_server.py
📝 Walkthrough

Walkthrough

Adds an opt-in adaptive layout and image-fitting system for plugins, exposes it through BasePlugin and src.common, extends manifest and font metadata, and adds multi-size preview, scale-up validation, compatibility warnings, documentation, and tests.

Changes

Adaptive layout system

Layer / File(s) Summary
Layout geometry, typography, and manifest contracts
schema/manifest_schema.json, src/adaptive_layout.py, src/font_manager.py, src/common/text_helper.py, src/common/logo_helper.py, docs/ADAPTIVE_LAYOUT.md, test/test_adaptive_layout.py
Defines responsive regions, font ladders, fitting APIs, composite layouts, design-size metadata, font cache invalidation, and related coverage.
Image fitting and plugin rendering APIs
src/adaptive_images.py, src/plugin_system/base_plugin.py, src/common/__init__.py, src/common/README.md, test/test_adaptive_images.py
Adds image fit modes, cached fitting, fitted text/image drawing, plugin layout caching, and public adaptive-layout exports.
Harness fill checks and size resolution
src/plugin_system/testing/{harness.py,sizes.py,bounds_display_manager.py,mocks.py,loading.py}, scripts/check_plugin.py, test/test_harness_fill.py
Adds scale-up coverage validation, variant-aware checking, size parsing and resolution, negative-coordinate recording, and fill-check tests.
Preview size presets and render matrix
scripts/dev_server.py, scripts/templates/dev_preview.html, docs/DEV_PREVIEW.md
Adds reusable rendering helpers, /api/sizes, /api/render-matrix, size presets, and an all-sizes preview gallery.
Compatibility checks and supporting guidance
src/plugin_system/plugin_loader.py, src/__init__.py, src/image_utils.py, docs/*, test/test_loader_compat_warning.py
Adds manifest minimum-version warnings, updates the package version, marks legacy image utilities deprecated, and documents adaptive layout usage.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.60% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main adaptive layout and image-fitting changes in the pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/adaptive-layout

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread scripts/dev_server.py
display_manager=display_manager,
cache_manager=cache_manager,
plugin_manager=plugin_manager,
install_deps=False,
Comment thread scripts/dev_server.py Fixed
Comment thread scripts/dev_server.py Fixed
Comment thread scripts/dev_server.py
mock_data, width, height, skip_update)
except Exception as e:
return jsonify({'error': f'Failed to load plugin: {e}'}), 500
return jsonify(result)
Comment thread scripts/dev_server.py Fixed
Comment thread scripts/dev_server.py
'render_time_ms': 0,
'errors': [f'Failed to load plugin: {e}'],
'warnings': []})
return jsonify({'results': results})
@codacy-production

codacy-production Bot commented Jul 11, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 high

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category Results
ErrorProne 2 high

View in Codacy

🟢 Metrics 245 complexity · 2 duplication

Metric Results
Complexity 245
Duplication 2

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/font_manager.py (1)

106-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

cache_generation isn't bumped by every cache-invalidating path, breaking its own contract.

The docstring at Line 107-108 says the counter is "bumped whenever cached font objects are invalidated," but only reload_config increments it. clear_cache() (and its callers set_override, remove_override, add_font, remove_font) also clear font_cache/metrics_cache without bumping cache_generation. BasePlugin.layout (src/plugin_system/base_plugin.py) uses this exact counter to decide whether to rebuild its memoized LayoutContext (and thus its _fit_cache), so a running plugin's adaptive-layout font-fit results will silently go stale after any font-override/add/remove operation from the web UI, only refreshing on process restart.

Bumping it inside clear_cache() itself would cover all four call sites in one place:

🐛 Proposed fix (outside the selected range, in `clear_cache()`)
def clear_cache(self):
    """Clear font and metrics cache."""
    self.font_cache.clear()
    self.metrics_cache.clear()
    self.cache_generation += 1
    logger.info("Font cache cleared")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/font_manager.py` around lines 106 - 121, Update FontManager.clear_cache()
to increment cache_generation whenever it clears font_cache and metrics_cache,
preserving the existing cache-clearing behavior and ensuring callers such as
set_override, remove_override, add_font, and remove_font invalidate dependent
caches consistently; avoid adding separate increments in those callers or
reload_config.
🧹 Nitpick comments (5)
src/common/logo_helper.py (1)

75-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Good fix — extract the shared default-size computation.

Computing max_width/max_height before the cache lookup correctly makes the cache size-qualified. However this same "default to display_* * 1.5" logic now exists in three places: here, _resize_logo (Line 228-231), and _create_placeholder_logo (Line 286-289). Consider extracting a small private helper (e.g. _effective_max_size(max_width, max_height)) to avoid future divergence.

♻️ Proposed refactor
def _effective_max_size(self, max_width: Optional[int], max_height: Optional[int]) -> Tuple[int, int]:
    """Resolve default max dimensions (display size * 1.5) when unset."""
    if max_width is None:
        max_width = int(self.display_width * 1.5)
    if max_height is None:
        max_height = int(self.display_height * 1.5)
    return max_width, max_height

Then call max_width, max_height = self._effective_max_size(max_width, max_height) in load_logo, _resize_logo, and _create_placeholder_logo.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/common/logo_helper.py` around lines 75 - 88, Extract the repeated
default-dimension logic into a private helper such as _effective_max_size,
preserving the display_width/display_height multiplied by 1.5 behavior for unset
values. Update load_logo, _resize_logo, and _create_placeholder_logo to resolve
max_width and max_height through this helper before using them.
src/plugin_system/base_plugin.py (1)

216-251: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update the plugin development guide for draw_image() docs/PLUGIN_DEVELOPMENT_GUIDE.md still says there is no draw_image() helper, but BasePlugin.draw_image() is now the supported adaptive-layout API, so the guidance should be aligned.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugin_system/base_plugin.py` around lines 216 - 251, Update
docs/PLUGIN_DEVELOPMENT_GUIDE.md to document BasePlugin.draw_image() as the
supported adaptive-layout image helper. Remove or revise guidance claiming no
draw_image() helper exists, and describe its fit/alignment behavior and relevant
options consistently with the method’s signature.

Source: Coding guidelines

src/adaptive_layout.py (1)

32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Unused imports flagged by static analysis in two new files. Both field (from dataclasses) in adaptive_layout.py and Union (from typing) in adaptive_images.py are imported but never referenced — all type annotations use Any, Tuple, or Optional instead.

  • src/adaptive_layout.py#L32: remove field from the dataclasses import.
  • src/adaptive_images.py#L28: remove Union from the typing import.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/adaptive_layout.py` at line 32, Remove the unused field import from the
dataclasses import in src/adaptive_layout.py (line 32), and remove the unused
Union import from the typing import in src/adaptive_images.py (line 28); leave
all referenced imports unchanged.

Source: Linters/SAST tools

src/plugin_system/testing/harness.py (1)

328-341: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Consider histogram() for the ink-ratio calculation.

sum(1 for p in lit.getdata() if p) iterates every pixel in Python. After thresholding, lit is binary (0 or 255), so lit.histogram()[255] gives the lit count in a single C-level call — faster and more idiomatic, with no generator overhead. The difference is negligible for small LED matrix images but aligns with resource-conscious practices.

♻️ Optional refactor
-    ink = sum(1 for p in lit.getdata() if p) / (image.width * image.height)
+    ink = lit.histogram()[255] / (image.width * image.height)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugin_system/testing/harness.py` around lines 328 - 341, Update the ink
calculation in fill_metrics to use lit.histogram()[255] instead of iterating
over lit.getdata(), preserving the existing normalization by image.width *
image.height and the returned metrics.
src/plugin_system/testing/mocks.py (1)

164-170: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Reuse the shared fallback FontManager here. FontManager({}) eagerly scans assets/fonts and reloads overrides in __init__, so each MockPluginManager() repeats that work; BasePlugin._get_font_manager() already falls back to the module-level singleton.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugin_system/testing/mocks.py` around lines 164 - 170, Update
MockPluginManager’s font_manager initialization to reuse the shared module-level
fallback FontManager instead of constructing FontManager({}) for every instance.
Preserve the existing exception fallback behavior and ensure
BasePlugin._get_font_manager() continues using the singleton when no manager is
available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/FONT_MANAGER.md`:
- Around line 3-6: Update the “Picking a size automatically” documentation to
scope self.layout.fit_text(...) explicitly to BasePlugin authors, or explain how
FontManager users obtain the required layout context; avoid directing generic
manager readers to an unavailable self.layout attribute.

In `@docs/PLUGIN_DEVELOPMENT_GUIDE.md`:
- Around line 5-8: Revise the “Rendering guidance” text in
PLUGIN_DEVELOPMENT_GUIDE.md to clarify that dynamic display sizing and adaptive
layouts apply only to plugins adopting the adaptive APIs. Explicitly preserve
classic rendering behavior as the default rather than implying all plugins must
migrate.

In `@scripts/dev_server.py`:
- Around line 277-287: Update the request handlers around _parse_render_request,
_render_once, and api_render_matrix to catch all expected parsing/rendering
exceptions, log the full exceptions server-side, and return only generic error
messages to API clients. Remove direct str(e), f-string exception interpolation,
and any raw exception text carried in result responses, while preserving the
existing HTTP status codes and successful response behavior.

---

Outside diff comments:
In `@src/font_manager.py`:
- Around line 106-121: Update FontManager.clear_cache() to increment
cache_generation whenever it clears font_cache and metrics_cache, preserving the
existing cache-clearing behavior and ensuring callers such as set_override,
remove_override, add_font, and remove_font invalidate dependent caches
consistently; avoid adding separate increments in those callers or
reload_config.

---

Nitpick comments:
In `@src/adaptive_layout.py`:
- Line 32: Remove the unused field import from the dataclasses import in
src/adaptive_layout.py (line 32), and remove the unused Union import from the
typing import in src/adaptive_images.py (line 28); leave all referenced imports
unchanged.

In `@src/common/logo_helper.py`:
- Around line 75-88: Extract the repeated default-dimension logic into a private
helper such as _effective_max_size, preserving the display_width/display_height
multiplied by 1.5 behavior for unset values. Update load_logo, _resize_logo, and
_create_placeholder_logo to resolve max_width and max_height through this helper
before using them.

In `@src/plugin_system/base_plugin.py`:
- Around line 216-251: Update docs/PLUGIN_DEVELOPMENT_GUIDE.md to document
BasePlugin.draw_image() as the supported adaptive-layout image helper. Remove or
revise guidance claiming no draw_image() helper exists, and describe its
fit/alignment behavior and relevant options consistently with the method’s
signature.

In `@src/plugin_system/testing/harness.py`:
- Around line 328-341: Update the ink calculation in fill_metrics to use
lit.histogram()[255] instead of iterating over lit.getdata(), preserving the
existing normalization by image.width * image.height and the returned metrics.

In `@src/plugin_system/testing/mocks.py`:
- Around line 164-170: Update MockPluginManager’s font_manager initialization to
reuse the shared module-level fallback FontManager instead of constructing
FontManager({}) for every instance. Preserve the existing exception fallback
behavior and ensure BasePlugin._get_font_manager() continues using the singleton
when no manager is available.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9bb2abf3-8b18-4887-8367-771f799d6c38

📥 Commits

Reviewing files that changed from the base of the PR and between 05e7c43 and 278d757.

📒 Files selected for processing (31)
  • docs/ADAPTIVE_LAYOUT.md
  • docs/ADVANCED_PLUGIN_DEVELOPMENT.md
  • docs/DEVELOPER_QUICK_REFERENCE.md
  • docs/DEV_PREVIEW.md
  • docs/FONT_MANAGER.md
  • docs/PLUGIN_API_REFERENCE.md
  • docs/PLUGIN_DEVELOPMENT_GUIDE.md
  • schema/manifest_schema.json
  • scripts/check_plugin.py
  • scripts/dev_server.py
  • scripts/templates/dev_preview.html
  • src/__init__.py
  • src/adaptive_images.py
  • src/adaptive_layout.py
  • src/common/README.md
  • src/common/__init__.py
  • src/common/logo_helper.py
  • src/common/text_helper.py
  • src/font_manager.py
  • src/image_utils.py
  • src/plugin_system/base_plugin.py
  • src/plugin_system/plugin_loader.py
  • src/plugin_system/testing/bounds_display_manager.py
  • src/plugin_system/testing/harness.py
  • src/plugin_system/testing/loading.py
  • src/plugin_system/testing/mocks.py
  • src/plugin_system/testing/sizes.py
  • test/test_adaptive_images.py
  • test/test_adaptive_layout.py
  • test/test_harness_fill.py
  • test/test_loader_compat_warning.py

Comment thread docs/FONT_MANAGER.md Outdated
Comment thread docs/PLUGIN_DEVELOPMENT_GUIDE.md Outdated
Comment thread scripts/dev_server.py
Chuck and others added 5 commits July 11, 2026 10:40
- docs: scope the self.layout note to BasePlugin subclasses (others build
  a LayoutContext directly) and make explicit that adaptive layout is
  opt-in — classic rendering stays unless a plugin adopts the APIs.
- dev_server: broaden the render-request catch (a bad manifest.json now
  returns a clean 400 instead of an unhandled 500) and stop echoing raw
  exception text in the loader-failure responses — full tracebacks go to
  the dev server's console log instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
CodeQL (py/path-injection): plugin_id arrives in request input and flows
into filesystem paths via find_plugin_dir. Gate it with the same
^[a-zA-Z0-9_-]{1,64}$ allowlist the web UI's pages_v3 uses, at the
single choke point every route resolves through.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
CodeQL doesn't recognize the interprocedural allowlist as a
path-injection barrier; add the canonical one — normalize (without
following symlinks, since dev plugins are commonly symlinked into
plugins/) and require the result to stay inside the search dir.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
CodeQL doesn't credit the sanitization inside find_plugin_dir along
this flow; apply its documented barrier (normpath + startswith against
the allowed roots) inline in _parse_render_request, on the exact path
that reaches the render/load sinks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
CodeQL's barrier-guard recognition doesn't see a startswith check
inside an any() comprehension, so the normalize-and-prefix approach
still flagged. Break the taint outright instead: after lookup, re-derive
the directory by enumerating the search dirs (iterdir) and matching by
path equality — the Path used for all downstream file access is built
solely from trusted listings, never from request input.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
@ChuckBuilds

Copy link
Copy Markdown
Owner Author

Note on the failing CodeQL / Codacy checks

The py/path-injection alert (#150) pre-dates this PRscripts/dev_server.py shipped with this lookup in #264, and the alert exists against main's analysis. It surfaces here because this PR touches the file.

This PR now actually fixes the underlying issue with three layers (commits 72b443d, 149f1da, a11c596):

  1. plugin_id is allowlisted (^[a-zA-Z0-9_-]{1,64}$, same pattern pages_v3 uses) before any path lookup.
  2. A normalize-and-containment check against the plugin search dirs.
  3. The directory used for all downstream file access is re-derived from trusted iterdir() listings — request-derived strings never enter its construction.

Traversal ids (../../etc, foo/../bar, oversized, non-string) are verified rejected; symlinked dev plugins still work (no symlink resolution in the containment logic).

CodeQL's taint tracking doesn't credit any of these as barriers on this flow (it flags the open() even when the path comes from a directory enumeration), so the alert will need a manual dismissal — I'd suggest "false positive" on alert 150 after eyeballing _trusted_plugin_dir. The two py/stack-trace-exposure warnings are the dev tool's deliberate errors[]/warnings[] arrays (exception messages, not tracebacks; localhost-bound tool whose purpose is showing plugin authors what broke) — same call as discussed in the review thread above.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants